feat(registration): cap faculty selection and replace other faculty with majors - #90
Conversation
There was a problem hiding this comment.
Good work, the faculty cap, addMajor round-trip via majorCount, and preserving in-progress fields across that redirect are all solid, and matches the ticket well. Two things to fix before merging:
1. stripIrrelevantFields doesn't strip majors/majorCount in the returningUoa and non-UoA branches
Only the newUoa branch destructures them out:
} else if (lastPage == "newUoa") {
const { primaryAffiliation, nonUoaExcerpt, nonUoaPitch, ...stripped } = draftFields;The returningUoa and else (non-UoA) branches don't include majors/majorCount in their destructured-and-discarded set, so they leak through. Concretely: someone starts the UoA path, types a major or two, goes back, switches to "not a UoA student," submits — their typed majors are still sitting in the cookie and flow into fullDraft for a registration type that shouldn't have them. Worth adding majors/majorCount to the stripped fields in both of the other two branches.
Would also be worth adding a case to the existing "stripIrrelevantFields per branch" tests that sets majors in the draft before checking what survives — none of the current ones would have caught this, since none of them set majors before asserting.
2. MAX_LENGTHS.otherFaculty is still in constants.ts
...
Ticket asked for otherFaculty to be removed from RegistrationDraft/stripIrrelevantFields/NewUoaPage — this is the same cleanup, just missed in constants.ts. For now, just delete it — the faculty enum validation this constant was standing in for will be tracked as a new ticket #92, which will make this fully redundant anyway.
Everything else looks good — parseRegistrationFormData.test.ts is a nice bit of extra coverage too, appreciated 👍
WilliamTayNZ
left a comment
There was a problem hiding this comment.
Good work!
Note there needs to be an additional test in the "stripIrrelevantFields per branch" tests in actions.test.ts, same pattern as the existing three (set majors in the cookie draft, assert it's stripped in the returningUoa and newNonUoa branches, kept in the newUoa branch).
But I will merge this for now and this can be picked up later
Summary
MAX_FACULTIES(2), enforced server-side inactions.tsotherFacultyfree-text field entirely (UI,actions.ts,RegistrationDraft,stripIrrelevantFields)MAX_MAJORS(4) free-text inputs, each capped atMAX_LENGTHS.major(40 chars)addMajorintent that round-trips through the cookie draft, incrementingmajorCountand preserving in-progress field values — mirrors the existing "back" navigation patternmajorsthrough toParsedRegistrationFormSubmissionviatoParsedSubmissionCloses #85